Skip to content

CSTACKEX-212: fix for snapshot failure for attached cs volumes nfs an…#77

Open
sathvikaragi wants to merge 5 commits into
mainfrom
bugfix/CSTACKEX-212
Open

CSTACKEX-212: fix for snapshot failure for attached cs volumes nfs an…#77
sathvikaragi wants to merge 5 commits into
mainfrom
bugfix/CSTACKEX-212

Conversation

@sathvikaragi

@sathvikaragi sathvikaragi commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Description

Fix snapshot failure for CloudStack volumes attached to running VMs on ONTAP primary storage (both NFS3 and iSCSI protocols).

This PR...

When a volume was created and attached to a running VM in a single step,by enabling create on storage and choose the storage pool tag the volume format was not being set correctly. The format is now determined by the hypervisor type (KVM → QCOW2) in volumeapiserviceimpl via [getImageFormatByHypervisor(HypervisorType] mirroring the [getSupportedImageFormatForCluster] in VolumeOrchestrator file.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

How Has This Been Tested?

Tested on a dev setup against these scenarios:

Scenario A — Attach data disk to running VM, then snapshot

  • Create ONTAP primary storage pool (NFS3 or iSCSI)
  • Deploy VM with data disk using pool-tagged disk offering → VM reaches Running state
  • Create volume attached to the running VM by enabling create on storage and choose the storage pool tag
  • Take snapshot of attached volume — ✅ succeeds (was failing before fix)

Scenario B — Attach volume to root-disk-only VM, then snapshot

  • Create ONTAP primary storage pool (NFS3 or iSCSI)
  • Deploy VM without data disk → VM reaches Running state
  • Create and attach volume to the running VM by enabling create on storage and choose the storage pool tag
  • Take snapshot of attached volume — ✅ succeeds (was failing before fix)

scenarios-C-create a volume on storage pool but not attach to any vm

  • Create ONTAP primary storage pool (NFS3 or iSCSI)
  • Create volume by enabling create on storage and choose the storage pool tag
  • Take snapshot- — ✅ succeeds (was failing before fix)

Screenshots (if appropriate):

snapshots for when cs volume is attached to NFS and ISCSI instance that has both root and data disk and create on storage enabled:
Screenshot 2026-07-15 at 12 28 32 AM
Screenshot 2026-07-15 at 12 28 38 AM

verifying on ontap and db:
Screenshot 2026-07-15 at 12 28 58 AM
Screenshot 2026-07-15 at 12 29 10 AM
Screenshot 2026-07-15 at 12 30 43 AM

snapshots for when cs volume is attached to NFS and ISCSI instance that has only root and also case when they are not attached to any instance and create on storage enabled:
Screenshot 2026-07-15 at 12 52 30 AM
Screenshot 2026-07-15 at 12 52 39 AM

How did you try to break this feature and the system with this change?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes snapshot failures for ONTAP-backed volumes that are created-and-attached to running VMs in a single step by ensuring the volume’s image format is set consistently (KVM → QCOW2) during ONTAP volume creation.

Changes:

  • Set VolumeVO.format during createAsync() for both iSCSI and NFS3 ONTAP-managed volumes based on the pool hypervisor.
  • Add unit-test stubbing for StoragePool.getHypervisor() to support the new format-resolution logic.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java Set volume format during creation and add hypervisor→image-format resolution helper.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java Update tests to provide pool hypervisor needed by the new format-setting logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sandeeplocharla sandeeplocharla left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work!!!

Copilot AI review requested due to automatic review settings July 10, 2026 10:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Comment thread server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
Comment thread server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
Comment thread server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
Copilot AI review requested due to automatic review settings July 14, 2026 19:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment on lines +1102 to +1108
VolumeVO volVO = _volsDao.findById(volumeInfo.getId());
if (volVO.getFormat() == null) {
HypervisorType hyperType = storagePool.getHypervisor();
volVO.setFormat(getSupportedImageFormatForHypervisor(hyperType));
}
_volsDao.update(volVO.getId(), volVO);
return volVO;
Copilot AI review requested due to automatic review settings July 14, 2026 20:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment on lines +1104 to +1106
HypervisorType hyperType = storagePool.getHypervisor();
ImageFormat format = getSupportedImageFormatForHypervisor(hyperType);
if (format != null) {
Comment on lines +1114 to +1127
private ImageFormat getSupportedImageFormatForHypervisor(HypervisorType hyperType) {
if (hyperType == HypervisorType.XenServer) {
return ImageFormat.VHD;
} else if (hyperType == HypervisorType.KVM) {
return ImageFormat.QCOW2;
} else if (hyperType == HypervisorType.VMware) {
return ImageFormat.OVA;
} else if (hyperType == HypervisorType.Ovm) {
return ImageFormat.RAW;
} else if (hyperType == HypervisorType.Hyperv) {
return ImageFormat.VHDX;
} else {
return null;
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants